Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concrete type parameters for types as type members #116

Merged
merged 4 commits into from
Jan 30, 2017

Conversation

j14159
Copy link
Collaborator

@j14159 j14159 commented Jan 29, 2017

Fixes #113

Lets us write:

type option 'a  = Some 'a | None
type int_opt = IntOpt option int

and actually use int_opt. Prior to this fix using int_opt, e.g. like IntOpt Some 1 would fail to type as it tried to unify t_int and undefined. We weren't capturing int as a value for a type variable.

cc/ @danabr for review as we've discussed this fix prior.

Not applied to module-qualified types yet.  This lets the following
actually work:

    type opt 'a = Some 'a | None
    type int_opt = opt int

Up until this commit any attempt to use `int_opt` would cause a
unification failure because we weren't capturing `int` as an actual
parameter and associated variable name.  Now for each parameter that is
a concrete type we synthesize a variable name for it.
Refering to module.type_name with concrete type parameters now works.
The exact same block to link concrete types to synthetic type variables
occurred twice, now it's a single function.
@danabr
Copy link
Contributor

danabr commented Jan 30, 2017

Haven't had time for proper review yet, but this fails to compile for me with function_clause in find_covering_type:

module int_opt

type opt 'a = Some 'a | None
type int_opt = opt int 
type indirect = Indirect int_opt

let deconstruct (Indirect opt) =
  match opt with
    (Some 1) -> :blah

Update:
This is a bug also present in master, because wrong order of arguments in 2 calls to unify in alpaca_typer:unify_adt, so not due to this PR. Fixing those makes the code above compile.

@danabr
Copy link
Contributor

danabr commented Jan 30, 2017

One thing I think causes more trouble than it helps us is that we use the same construct #alpaca_type{} to refer to two different things: type definition and type application.

type int_opt = opt int is parsed as:

{alpaca_type,4,int_opt,
              {type_name,4,"int_opt"},
              [],
              [{alpaca_type,4,undefined,
                   {type_name,4,"opt"},
                   [{{type_var,4,":SynthTypeVar_0"},t_int}],
                   [t_int]}]}

Perhaps we should rather parse it as:

{alpaca_type_def,4,int_opt,
              {type_name,4,"int_opt"},
              [],
              [{alpaca_type_app,4,undefined,
                   {type_name,4,"opt"},
                   [t_int]}]}

i.e. a type application takes some type arguments (in this case t_int). When seeing the type application, the typer then looks up opt and checks that the correct number of type arguments are given, and assigns them to the proper type variables of opt.

You might end up with the same end result when you do as you do here, but it is more confusing (at least to me), to have #alpaca_type{} sometimes represent type application, and sometimes type definition.

@j14159: These are just opinions. You've got a +1 to merge if you think your approach makes more sense.

@j14159
Copy link
Collaborator Author

j14159 commented Jan 30, 2017

Good catch on that example, thanks! I've added it as a test and will fix that, it looks like somehow we're trying to unify t_int and a line number, definite bug :D

I think you make a good point with def vs app although I'm inclined to think of them more as binding a name to a type vs an actual type. I think this distinction is pretty similar to what we'll need to do to add lambdas/anonymous functions, e.g. separate the idea of a function from the action of binding it. Thoughts?

Unifying two different ADTs when one is a member of the other called
unify/4 with the incorrect order of arguments.
@j14159
Copy link
Collaborator Author

j14159 commented Jan 30, 2017

@danabr I opened issue #117 to talk about AST changes as I think you raised a good point about defs vs types themselves that relates quite a bit to functions too.

@j14159 j14159 merged commit a39cdff into alpaca-lang:master Jan 30, 2017
@j14159 j14159 deleted the fix-113 branch January 30, 2017 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specifying concrete types as type parameters instead of vars fails typing
2 participants